expression: fix the arg verification for json functions. #54145
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ti-chi-bot
bot
added
release-note
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #54145 +/- ##
================================================
+ Coverage 72.8582% 73.3582% +0.4999%
================================================
Files 1672 1676 +4
Lines 462955 468918 +5963
================================================
+ Hits 337301 343990 +6689
+ Misses 104875 104169 -706
+ Partials 20779 20759 -20
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
/hold Also consider #54044 (comment). Maybe it's not good to verify args type/charset in |
ti-chi-bot
bot
added
the
do-not-merge/hold
dveeden
reviewed
Jun 21, 2024
|
PR needs rebase. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
ti-chi-bot
bot
added
the
needs-rebase
|
@YangKeao can you update this PR? |
YangKeao
force-pushed
the
fix-54029
branch
2 times, most recently
from
8371d26 to
51db52c
Compare
Signed-off-by: Yang Keao <[email protected]>
Done. PTAL |
|
/retest |
|
/unhold |
ti-chi-bot
bot
removed
the
do-not-merge/hold
YangKeao
removed
the
needs-rebase
YangKeao
added
the
needs-cherry-pick-release-8.5
ti-chi-bot
bot
added
do-not-merge/needs-triage-completed
and removed
do-not-merge/needs-triage-completed
labels
Defined2014
reviewed
Nov 20, 2024
ti-chi-bot
bot
added
approved
needs-1-more-lgtm
ti-chi-bot
bot
added
lgtm
and removed
needs-1-more-lgtm
dveeden
requested changes
Nov 20, 2024
There was a problem hiding this comment.
CREATE TABLE t1(id INT PRIMARY KEY, d1 DATE, d2 DATETIME, t1 TIME, t2 TIMESTAMP, b1 BIT, b2 BINARY);
INSERT INTO t1 VALUES (1, '2024-06-14', '2024-06-14 09:37:00', '09:37:00', '2024-06-14 09:37:00', b'0', 0x41);
SELECT JSON_TYPE(b2) FROM t1;TiDB:
mysql-8.0.11-TiDB-v8.5.0-alpha-160-gce19496ae9-dirty> SELECT JSON_TYPE(b2) FROM t1;
ERROR 3144 (HY000): Cannot create a JSON value from a string with CHARACTER SET 'binary'.
MySQL 9.1.0:
mysql-9.1.0> SELECT JSON_TYPE(b2) FROM t1;
ERROR 3144 (22032): Cannot create a JSON value from a string with CHARACTER SET 'binary'.
The SQL State is 22032 for MySQL and HY000 for TiDB
diff --git a/pkg/parser/mysql/errcode.go b/pkg/parser/mysql/errcode.go
index 05d5dc3e69..bfbd196138 100644
--- a/pkg/parser/mysql/errcode.go
+++ b/pkg/parser/mysql/errcode.go
@@ -898,6 +898,7 @@ const (
ErrInvalidJSONText = 3140
ErrInvalidJSONTextInParam = 3141
ErrInvalidJSONPath = 3143
+ ErrInvalidJSONCharset = 3144
ErrInvalidTypeForJSON = 3146
ErrInvalidJSONPathWildcard = 3149
ErrInvalidJSONContainsPathType = 3150
diff --git a/pkg/parser/mysql/errname.go b/pkg/parser/mysql/errname.go
index f757f825da..845c50a21b 100644
--- a/pkg/parser/mysql/errname.go
+++ b/pkg/parser/mysql/errname.go
@@ -907,6 +907,7 @@ var MySQLErrName = map[uint16]*ErrMessage{
ErrInvalidJSONText: Message("Invalid JSON text: %-.192s", nil),
ErrInvalidJSONTextInParam: Message("Invalid JSON text in argument %d to function %s: \"%s\" at position %d.", nil),
ErrInvalidJSONPath: Message("Invalid JSON path expression %s.", nil),
+ ErrInvalidJSONCharset: Message("Cannot create a JSON value from a string with CHARACTER SET '%s'.", nil),
ErrInvalidTypeForJSON: Message("Invalid data type for JSON data in argument %d to function %s; a JSON string or JSON type is required.", nil),
ErrInvalidJSONPathWildcard: Message("In this situation, path expressions may not contain the * and ** tokens or an array range.", nil),
ErrInvalidJSONContainsPathType: Message("The second argument can only be either 'one' or 'all'.", nil),
diff --git a/pkg/parser/mysql/state.go b/pkg/parser/mysql/state.go
index 2cbc6f1d2b..307965d088 100644
--- a/pkg/parser/mysql/state.go
+++ b/pkg/parser/mysql/state.go
@@ -254,6 +254,7 @@ var MySQLState = map[uint16]string{
ErrInvalidJSONText: "22032",
ErrInvalidJSONTextInParam: "22032",
ErrInvalidJSONPath: "42000",
+ ErrInvalidJSONCharset: "22032",
ErrInvalidJSONData: "22032",
ErrInvalidJSONPathWildcard: "42000",
ErrJSONUsedAsKey: "42000", |
Nice catch! Fixed. PTAL @dveeden |
dveeden
approved these changes
Nov 20, 2024
|
/cc @tangenta |
tangenta
approved these changes
Nov 20, 2024
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Defined2014, dveeden, tangenta, windtalker The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
ti-chi-bot
bot
added
approved
lgtm
and removed
needs-1-more-lgtm
[LGTM Timeline notifier]Timeline:
|
Defined2014
reviewed
Nov 20, 2024
Signed-off-by: Yang Keao <[email protected]>
|
/retest |
1 similar comment
|
/retest |
|
In response to a cherrypick label: new pull request created to branch |
5 tasks
budney
pushed a commit
to budney/tidb
that referenced
this pull request
May 18, 2025
13 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What problem does this PR solve?
Issue Number: close #54029, close #54044
Problem Summary:
For some json functions, the arg verification is not implemented correctly:
json_type, it didn't verify the type of args.For
PREPARE, the param marker is set to binary string type, which is not expected. This PR will give it a default collation. It will not affect theEXECUTE, because when it's executing a statement, it'll infer the collation according to the datum (whose collation is set according to the collation settings of the connection).What changed and how does it work?
Check List
Tests
Release note